[Utilities] fix ProductOfSets to handle zero-dimensional rows#3022
[Utilities] fix ProductOfSets to handle zero-dimensional rows#3022odow wants to merge 8 commits into
Conversation
|
Okay, I'm happy with this. It's just simpler all round. It now has one |
| VAF = MOI.VectorAffineFunction{Float64} | ||
| indices = MOI.get(sets, MOI.ListOfConstraintIndices{VAF,S}()) | ||
| @test indices == MOI.ConstraintIndex{VAF,S}.([1, 3, 6, 8]) | ||
| @test indices == MOI.ConstraintIndex{VAF,S}.([1, 2, 3, 4]) |
There was a problem hiding this comment.
The downside is that you might argue this is breaking, because we change the constraint indices. But I don't think we ever document that the index values are part of the public API. We might re-order them in any way. They just need to make sure that we iterate over them in the order that they were added (within a constraint type).
|
We can fix SCS, but I've also added a backward compatible https://github.com/jump-dev/MathOptInterface.jl/actions/runs/29459997140 |
|
Sigh. DiffOpt has: https://github.com/jump-dev/DiffOpt.jl/blob/master/src/product_of_sets.jl This is why we shouldn't have all these tricky little helpers everywhere. Consumers use them in all sorts of unexpected ways and we get boxed into how we can support them. |
|
@blegat we should chat about this. I don't really know how to get these simplifications in MOI without breaking DiffOpt. We probably need to keep the current fields and just add some additional bookkeeping for the zero-dim rows. And then also go and fix this in DiffOpt. |
But that won't work because DiffOpt won't have those fields... I've opened a PR in DiffOpt to remove the usage: jump-dev/DiffOpt.jl#372 |
Closes #3019